<html>
<head><meta charset="utf-8"><title>Uplift the `invalid_atomic_ordering` lint from clippy · t-compiler/major changes · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/index.html">t-compiler/major changes</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html">Uplift the `invalid_atomic_ordering` lint from clippy</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="218376118"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218376118" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218376118">(Dec 01 2020 at 03:12)</a>:</h4>
<p>A new proposal has been announced: <a href="https://github.com/rust-lang/compiler-team/issues/390">Uplift the <code>invalid_atomic_ordering</code> lint from clippy to rustc</a>. I have no clue if this will be discussed in a meeting or not but you probably can find better things to do with that time in any case.</p>
<p><a href="#narrow/stream/242791-t-infra/topic/What.20has.20happened.20to.20the.20MCP.20zulip.20stream.20creation.20robot.3F">Something seems to have happened</a> to the MCP stream robot, so I am fulfilling its role manually. Beep boop.</p>



<a name="218380032"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218380032" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> BlackHoleFox <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218380032">(Dec 01 2020 at 04:35)</a>:</h4>
<p>I'm in favor /o/. A. It seems harmless and provides better IDE checking vs a runtime panic, and B. This bit me the other day and would have been nice to have :)</p>



<a name="218423088"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218423088" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218423088">(Dec 01 2020 at 13:45)</a>:</h4>
<p>I believe lint additions should be lang MCP?</p>



<a name="218462829"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218462829" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218462829">(Dec 01 2020 at 18:32)</a>:</h4>
<p>In <a href="#narrow/stream/213817-t-lang/topic/Process.20for.20moving.20lints.20from.20Clippy.20to.20Rustc/near/214891748">https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Process.20for.20moving.20lints.20from.20Clippy.20to.20Rustc/near/214891748</a> I was told to make it in compiler-team. <a href="https://github.com/rust-lang/compiler-team/issues/346">https://github.com/rust-lang/compiler-team/issues/346</a> is also precedent of this being a compiler-team thing in the past.</p>



<a name="218463202"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218463202" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218463202">(Dec 01 2020 at 18:35)</a>:</h4>
<p>(This would be a good thing to have a meta discussion about, I think -- personally I don't think lang needs to approve lints about things that don't work.  Maybe if there are proposals to lint on things that _do_ work it makes some sense to loop in lang because it's a weak language feature deprecation.  But especially for "that's the wrong way to use a library function" I don't think lang needs to be involved, though maybe libs would want to know.)</p>



<a name="218463437"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218463437" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> simulacrum <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218463437">(Dec 01 2020 at 18:37)</a>:</h4>
<p>Yeah, I think there's definitely some meta discussion -- my impression was that lang wanted to approve <em>all</em> lints. It seems fine for some of that to be ceded to T-compiler/libs.</p>



<a name="218480883"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218480883" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218480883">(Dec 01 2020 at 21:00)</a>:</h4>
<p>We have typically said we want to approve lints, yes.</p>



<a name="218480911"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218480911" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218480911">(Dec 01 2020 at 21:00)</a>:</h4>
<p>But I don't know that we need an MCP</p>



<a name="218480921"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218480921" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218480921">(Dec 01 2020 at 21:00)</a>:</h4>
<p>I might be inclined to just have the PR</p>



<a name="218480929"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218480929" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> nikomatsakis <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218480929">(Dec 01 2020 at 21:00)</a>:</h4>
<p>we should document it, regardless</p>



<a name="218483957"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218483957" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Josh Triplett <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218483957">(Dec 01 2020 at 21:26)</a>:</h4>
<p><span aria-label="+1" class="emoji emoji-1f44d" role="img" title="+1">:+1:</span> for just doing an FCP in the PR for lang team approval.</p>



<a name="218616435"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218616435" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> scottmcm <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218616435">(Dec 02 2020 at 20:41)</a>:</h4>
<p>For lack of an obvious place to put it, I filed it as a meeting proposal in <a href="https://github.com/rust-lang/lang-team/issues/72">https://github.com/rust-lang/lang-team/issues/72</a></p>



<a name="218647177"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218647177" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218647177">(Dec 03 2020 at 02:27)</a>:</h4>
<p>I just went and did it <a href="https://github.com/rust-lang/rust/pull/79654">https://github.com/rust-lang/rust/pull/79654</a>.</p>



<a name="218647808"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218647808" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218647808">(Dec 03 2020 at 02:41)</a>:</h4>
<p>One super nitpickey issue I thought of is:</p>
<p>When given <code>_.store(_, AcqRel)</code> the lint's help message tells you which orderings would be valid for a <code>store</code>, for example:</p>
<div class="codehilite"><pre><span></span><code>error: atomic stores cannot have `Acquire` or `AcqRel` ordering
  --&gt; $DIR/lint-invalid-atomic-ordering-bool.rs:25:20
   |
LL |     x.store(false, Ordering::AcqRel);
   |                    ^^^^^^^^^^^^^^^^
   |
   = help: consider using ordering modes `Release`, `SeqCst` or `Relaxed`
</code></pre></div>
<p>If the code <em>really</em> did want <code>AcqRel</code> semantics on the store, the correct answer would be <code>_.swap(_, AcqRel)</code>.</p>
<p>That said, I feel like anybody who knows that they <em>really</em> want <code>AcqRel</code> for that store is likely to know this already, and we suggest <code>SeqCst</code> which is strictly stronger on the store than the <code>AcqRel</code> swap would be anyway.</p>
<p>A similar issue also exists for a <code>_.load(Release)</code> since in the unlikely event where they <em>really</em> want that, the right answer would be to do both <code>_.load(Relaxed)</code> <em>and</em> a <code>fence(Release)</code> (and in practice, IME <code>_.load(Release)</code> is usually <code>_.load(&lt;uh, was it Acquire or Release that I use for loads?&gt;)</code>)</p>
<p>Anyway, I think this is fine, since the help message mentions sufficiently strong options for both those cases, and code that wants those things is pretty weird anyway and would probably only be written by people who know this already (hopefully). I figured I'd mention it in case someone strongly disagrees, though.</p>



<a name="218647984"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218647984" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Mario Carneiro <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218647984">(Dec 03 2020 at 02:44)</a>:</h4>
<p>Doesn't this ordering literally not make sense though? Like it makes no sense to give this to LLVM as it doesn't mean anything</p>



<a name="218648079"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218648079" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Mario Carneiro <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218648079">(Dec 03 2020 at 02:46)</a>:</h4>
<blockquote>
<p>If the code really did want AcqRel semantics on the store, </p>
</blockquote>
<p>That is, I don't think there is such a thing as AcqRel semantics on a store</p>



<a name="218648862"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218648862" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218648862">(Dec 03 2020 at 03:03)</a>:</h4>
<p><span class="user-mention" data-user-id="271719">@Mario Carneiro</span> right, it would need to be an RMW.</p>



<a name="218649133"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218649133" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218649133">(Dec 03 2020 at 03:09)</a>:</h4>
<p>you see this in implementation of some concurrent algorithms (petersons algorithm for example requires it when locking, i believe)</p>



<a name="218651214"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218651214" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Mario Carneiro <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218651214">(Dec 03 2020 at 03:54)</a>:</h4>
<p>If it's a RMW or something where the ordering makes sense, I would expect the error/lint warning against it not to exist</p>



<a name="218651319"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218651319" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218651319">(Dec 03 2020 at 03:56)</a>:</h4>
<p>Yeah, the lint doesn't fire on anything that wouldn't cause an unconditional panic.</p>



<a name="218651326"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/233931-t-compiler/major%20changes/topic/Uplift%20the%20%60invalid_atomic_ordering%60%20lint%20from%20clippy/near/218651326" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Thom Chiovoloni <a href="https://rust-lang.github.io/zulip_archive/stream/233931-t-compiler/major-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy.html#218651326">(Dec 03 2020 at 03:56)</a>:</h4>
<p>Anyway, it seems what I said was just very confusing, sorry.</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>